dont use gnu specific grep #885
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Hi @brian-level ! Thanks for submitting the PR!
Theoretically, you shouldn't have run into this issue if you ran ./script/bootstrap on your system since it installs GNU grep. Every system is different though, so maybe I missed something. Either way, replacing -oP with bash-native parsing is an improvement. Could you remove grep from script/bootstrap in this PR as well?
Also, please sign the CLA.
CLA I had signed earlier for a PR to openthread. Maybe it'll apply here? Question: in "openthread/scripts/check-api-version" it seems like the line 59
Will flag the normal case since the grep looks for [-+]#define so will have 2 lines? I think that == should be a != ? |
@brian-level , yes it applies here as well. Thanks. |
Please see above question. not sure about it. I will make a new PR in openthread/openthread with the above script change I think I need to leave grep in the bootstrap. There could be cases where people on non-Macs need it, or on Macs if they haven't installed the xcode commandline tools perhaps. The issue is that newer mac os doesnt allow even sudo access to /usr/bin and other places where things like grep are installed by Xcode which override brew apps. I have this issue with cmake as well. Using simpler greps is better IMHO anyway but there was really nothing wrong with the scripts except for Apple deciding to be Apple |
grep -oP is specific to GNU versions of grep which aren't (any longer) used on Mac/OSX platforms. The use in scripts is a bit overkill for just getting a token. This replaces grep with bash-native parsing